Add peer tags, span kind and trace root flag to MetricKey bucket #9178
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 14 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055530
Total [baseline] (8.612 s) : 0, 8611797
Agent [candidate] (1.071 s) : 0, 1071482
Total [candidate] (8.675 s) : 0, 8675202
section iast
Agent [baseline] (1.203 s) : 0, 1202651
Total [baseline] (9.439 s) : 0, 9438509
Agent [candidate] (1.182 s) : 0, 1181732
Total [candidate] (9.334 s) : 0, 9334163
gantt
title insecure-bank - break down per module: candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.457 ms) : 0, 1457
crashtracking [candidate] (1.479 ms) : 0, 1479
BytebuddyAgent [baseline] (738.2 ms) : 0, 738200
BytebuddyAgent [candidate] (748.717 ms) : 0, 748717
GlobalTracer [baseline] (243.732 ms) : 0, 243732
GlobalTracer [candidate] (248.083 ms) : 0, 248083
AppSec [baseline] (30.388 ms) : 0, 30388
AppSec [candidate] (30.914 ms) : 0, 30914
Debugger [baseline] (6.113 ms) : 0, 6113
Debugger [candidate] (6.169 ms) : 0, 6169
Remote Config [baseline] (679.225 µs) : 0, 679
Remote Config [candidate] (689.572 µs) : 0, 690
Telemetry [baseline] (13.898 ms) : 0, 13898
Telemetry [candidate] (14.071 ms) : 0, 14071
section iast
crashtracking [baseline] (1.474 ms) : 0, 1474
crashtracking [candidate] (1.45 ms) : 0, 1450
BytebuddyAgent [baseline] (868.573 ms) : 0, 868573
BytebuddyAgent [candidate] (852.174 ms) : 0, 852174
GlobalTracer [baseline] (237.118 ms) : 0, 237118
GlobalTracer [candidate] (234.095 ms) : 0, 234095
IAST [baseline] (29.913 ms) : 0, 29913
IAST [candidate] (29.565 ms) : 0, 29565
AppSec [baseline] (28.371 ms) : 0, 28371
AppSec [candidate] (27.821 ms) : 0, 27821
Debugger [baseline] (6.759 ms) : 0, 6759
Debugger [candidate] (6.669 ms) : 0, 6669
Remote Config [baseline] (628.062 µs) : 0, 628
Remote Config [candidate] (611.107 µs) : 0, 611
Telemetry [baseline] (8.64 ms) : 0, 8640
Telemetry [candidate] (8.366 ms) : 0, 8366
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.048 s) : 0, 1047841
Total [baseline] (10.749 s) : 0, 10749144
Agent [candidate] (1.046 s) : 0, 1045556
Total [candidate] (10.71 s) : 0, 10709638
section appsec
Agent [baseline] (1.224 s) : 0, 1223709
Total [baseline] (10.777 s) : 0, 10777022
Agent [candidate] (1.224 s) : 0, 1223968
Total [candidate] (10.743 s) : 0, 10742523
section iast
Agent [baseline] (1.179 s) : 0, 1179239
Total [baseline] (10.875 s) : 0, 10875060
Agent [candidate] (1.188 s) : 0, 1188487
Total [candidate] (10.908 s) : 0, 10907731
section profiling
Agent [baseline] (1.196 s) : 0, 1196060
Total [baseline] (10.89 s) : 0, 10890372
Agent [candidate] (1.215 s) : 0, 1214774
Total [candidate] (10.897 s) : 0, 10896531
gantt
title petclinic - break down per module: candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.456 ms) : 0, 1456
crashtracking [candidate] (1.455 ms) : 0, 1455
BytebuddyAgent [baseline] (732.356 ms) : 0, 732356
BytebuddyAgent [candidate] (732.475 ms) : 0, 732475
GlobalTracer [baseline] (242.346 ms) : 0, 242346
GlobalTracer [candidate] (242.337 ms) : 0, 242337
AppSec [baseline] (30.215 ms) : 0, 30215
AppSec [candidate] (30.094 ms) : 0, 30094
Debugger [baseline] (6.048 ms) : 0, 6048
Debugger [candidate] (6.015 ms) : 0, 6015
Remote Config [baseline] (666.947 µs) : 0, 667
Remote Config [candidate] (665.531 µs) : 0, 666
Telemetry [baseline] (13.758 ms) : 0, 13758
Telemetry [candidate] (11.436 ms) : 0, 11436
section appsec
crashtracking [baseline] (1.45 ms) : 0, 1450
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (756.008 ms) : 0, 756008
BytebuddyAgent [candidate] (756.572 ms) : 0, 756572
GlobalTracer [baseline] (235.398 ms) : 0, 235398
GlobalTracer [candidate] (235.366 ms) : 0, 235366
IAST [baseline] (23.738 ms) : 0, 23738
IAST [candidate] (23.611 ms) : 0, 23611
AppSec [baseline] (170.354 ms) : 0, 170354
AppSec [candidate] (168.791 ms) : 0, 168791
Debugger [baseline] (5.741 ms) : 0, 5741
Debugger [candidate] (6.575 ms) : 0, 6575
Remote Config [baseline] (644.074 µs) : 0, 644
Remote Config [candidate] (641.411 µs) : 0, 641
Telemetry [baseline] (9.184 ms) : 0, 9184
Telemetry [candidate] (9.878 ms) : 0, 9878
section iast
crashtracking [baseline] (1.455 ms) : 0, 1455
crashtracking [candidate] (1.454 ms) : 0, 1454
BytebuddyAgent [baseline] (851.288 ms) : 0, 851288
BytebuddyAgent [candidate] (858.479 ms) : 0, 858479
GlobalTracer [baseline] (234.278 ms) : 0, 234278
GlobalTracer [candidate] (234.501 ms) : 0, 234501
IAST [baseline] (30.868 ms) : 0, 30868
IAST [candidate] (30.45 ms) : 0, 30450
AppSec [baseline] (24.96 ms) : 0, 24960
AppSec [candidate] (26.076 ms) : 0, 26076
Debugger [baseline] (6.551 ms) : 0, 6551
Debugger [candidate] (7.457 ms) : 0, 7457
Remote Config [baseline] (607.773 µs) : 0, 608
Remote Config [candidate] (603.706 µs) : 0, 604
Telemetry [baseline] (8.279 ms) : 0, 8279
Telemetry [candidate] (8.312 ms) : 0, 8312
section profiling
crashtracking [baseline] (1.414 ms) : 0, 1414
crashtracking [candidate] (1.441 ms) : 0, 1441
BytebuddyAgent [baseline] (762.462 ms) : 0, 762462
BytebuddyAgent [candidate] (774.082 ms) : 0, 774082
GlobalTracer [baseline] (221.993 ms) : 0, 221993
GlobalTracer [candidate] (224.952 ms) : 0, 224952
AppSec [baseline] (30.008 ms) : 0, 30008
AppSec [candidate] (30.673 ms) : 0, 30673
Debugger [baseline] (7.067 ms) : 0, 7067
Debugger [candidate] (6.4 ms) : 0, 6400
Remote Config [baseline] (728.518 µs) : 0, 729
Remote Config [candidate] (711.907 µs) : 0, 712
Telemetry [baseline] (15.464 ms) : 0, 15464
Telemetry [candidate] (16.462 ms) : 0, 16462
ProfilingAgent [baseline] (107.422 ms) : 0, 107422
ProfilingAgent [candidate] (109.754 ms) : 0, 109754
Profiling [baseline] (108.053 ms) : 0, 108053
Profiling [candidate] (110.409 ms) : 0, 110409
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 1 performance regressions! Performance is the same for 10 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section baseline
no_agent (36.622 ms) : 36332, 36912
. : milestone, 36622,
appsec (45.852 ms) : 45455, 46249
. : milestone, 45852,
code_origins (45.452 ms) : 45049, 45855
. : milestone, 45452,
iast (45.092 ms) : 44708, 45477
. : milestone, 45092,
profiling (48.819 ms) : 48367, 49271
. : milestone, 48819,
tracing (43.345 ms) : 42992, 43698
. : milestone, 43345,
section candidate
no_agent (36.573 ms) : 36284, 36862
. : milestone, 36573,
appsec (48.197 ms) : 47767, 48626
. : milestone, 48197,
code_origins (44.484 ms) : 44091, 44877
. : milestone, 44484,
iast (44.367 ms) : 43980, 44755
. : milestone, 44367,
profiling (48.086 ms) : 47627, 48544
. : milestone, 48086,
tracing (43.505 ms) : 43145, 43865
. : milestone, 43505,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section baseline
no_agent (4.346 ms) : 4291, 4401
. : milestone, 4346,
iast (9.172 ms) : 9023, 9320
. : milestone, 9172,
iast_FULL (13.823 ms) : 13549, 14097
. : milestone, 13823,
iast_GLOBAL (10.066 ms) : 9891, 10240
. : milestone, 10066,
profiling (9.032 ms) : 8889, 9175
. : milestone, 9032,
tracing (7.743 ms) : 7633, 7853
. : milestone, 7743,
section candidate
no_agent (4.422 ms) : 4371, 4473
. : milestone, 4422,
iast (9.35 ms) : 9193, 9507
. : milestone, 9350,
iast_FULL (13.917 ms) : 13639, 14195
. : milestone, 13917,
iast_GLOBAL (10.066 ms) : 9882, 10251
. : milestone, 10066,
profiling (8.418 ms) : 8290, 8546
. : milestone, 8418,
tracing (7.689 ms) : 7580, 7799
. : milestone, 7689,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section baseline
no_agent (15.525 s) : 15525000, 15525000
. : milestone, 15525000,
appsec (14.799 s) : 14799000, 14799000
. : milestone, 14799000,
iast (18.741 s) : 18741000, 18741000
. : milestone, 18741000,
iast_GLOBAL (18.003 s) : 18003000, 18003000
. : milestone, 18003000,
profiling (15.529 s) : 15529000, 15529000
. : milestone, 15529000,
tracing (14.997 s) : 14997000, 14997000
. : milestone, 14997000,
section candidate
no_agent (15.495 s) : 15495000, 15495000
. : milestone, 15495000,
appsec (14.761 s) : 14761000, 14761000
. : milestone, 14761000,
iast (18.272 s) : 18272000, 18272000
. : milestone, 18272000,
iast_GLOBAL (17.973 s) : 17973000, 17973000
. : milestone, 17973000,
profiling (15.294 s) : 15294000, 15294000
. : milestone, 15294000,
tracing (15.104 s) : 15104000, 15104000
. : milestone, 15104000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~bcf4d04f2b, baseline=1.53.0-SNAPSHOT~5aa25baa32
dateFormat X
axisFormat %s
section baseline
no_agent (1.48 ms) : 1468, 1492
. : milestone, 1480,
appsec (3.674 ms) : 3457, 3891
. : milestone, 3674,
iast (2.202 ms) : 2139, 2265
. : milestone, 2202,
iast_GLOBAL (2.245 ms) : 2182, 2308
. : milestone, 2245,
profiling (2.04 ms) : 1990, 2091
. : milestone, 2040,
tracing (2.028 ms) : 1979, 2077
. : milestone, 2028,
section candidate
no_agent (1.479 ms) : 1468, 1491
. : milestone, 1479,
appsec (3.656 ms) : 3441, 3870
. : milestone, 3656,
iast (2.199 ms) : 2136, 2262
. : milestone, 2199,
iast_GLOBAL (2.245 ms) : 2181, 2308
. : milestone, 2245,
profiling (2.059 ms) : 2007, 2112
. : milestone, 2059,
tracing (2.02 ms) : 1971, 2069
. : milestone, 2020,
|
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
8c4a17b to
3e3c914
Compare
bric3
commented
Jul 16, 2025
Comment on lines
-115
to
-120
| 1 * writer.add(new MetricKey("resource", "service", "operation", "type", HTTP_OK, false), _) >> { MetricKey key, AggregateMetric value -> | ||
| value.getHitCount() == 1 && value.getTopLevelCount() == 1 && value.getDuration() == 100 | ||
| } |
There was a problem hiding this comment.
1 * writer.add(…, _) >> { MetricKey key, AggregateMetric value -> some assertions }FYI, these spock code are incorrect, it tries to verify the AggregateMetric, but actually nothing is properly verified, because >> indicates a stub, which is incorrect since it appears in the then section with a verification.
Instead, the verification code should be written this way, with an argument constraint :
1 * writer.add(…, { AggregateMetric value -> some assertions })
amarziali
reviewed
Jul 16, 2025
| for (String peerTag : features.peerTags()) { | ||
| Object value = span.getTag(peerTag); | ||
| if (value != null) { | ||
| peerTags.add(peerTag + ":" + TraceUtils.normalizeTag(value.toString())); |
There was a problem hiding this comment.
I would avoid creating a lot of new strings here. Instead we can return a map view of the tracer tags (a view returning only entries whose key is in peerTags) and write directly key then : then normalize(value) on the messagepack writer. It will avoid create a string here.
There was a problem hiding this comment.
A simplified version is just to hold key, value here (so that we can use them for the hashtag) but not concat the key:value
There was a problem hiding this comment.
Thanks, we're thinking alike!
I had the same thought about String creation. I tried a StringBuilder approach, but without notable benefit in the end on the serialization side, especially since UTF8BytesString actually creates a full String anyway.
The issue with keeping a map view, is it requires to create UTF8BytesString each time this key is serialized. I'll try to come up with a solution that balances both.
There was a problem hiding this comment.
Yes as an idea that can split in a couple of things.
First the need not to concatenate strings. For this you can just store the key in a list of UTF8ByteString (since they peerTags are quite stable they can be converted once anyway) and value in of List<String> and in SerializingMetricWriter just write first the key then : than the normalised value.
I don't think that the values can be efficiently cached since we don't know the cardinality in advance. Converting in UTF8 each time should be fine for now
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
3e3c914 to
48a0722
Compare
amarziali
reviewed
Jul 17, 2025
| StringBuilder peerTagBuilder = new StringBuilder(); | ||
| for (Map.Entry<String, String> peerTag : peerTags.entrySet()) { | ||
| peerTagBuilder.setLength(0); | ||
| String toWrite = peerTagBuilder |
There was a problem hiding this comment.
I'm wondering if we can do something to cache them with a DDCache (like 1024 entries assuming that it will be enough for peer tags)
There was a problem hiding this comment.
Indeed, that's something on my mind, but there some challenges there that I'd rather work in another smaller PR.
amarziali
approved these changes
Jul 17, 2025
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
48a0722 to
2d089ec
Compare
6 tasks
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
2d089ec to
696ab1c
Compare
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
3 times, most recently
from
b9006af to
b7aa90d
Compare
bric3
commented
Jul 30, 2025
dd-trace-core/src/test/groovy/datadog/trace/common/metrics/FootprintForkedTest.groovy
Outdated
Show resolved
Hide resolved
|
System tests are failing, need to wait on DataDog/system-tests#5002 |
bric3
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
c394992 to
b53a739
Compare
|
🎯 Code Coverage 🔗 Commit SHA: bcf4d04 | Docs | Was this helpful? Give us feedback! |
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
a5d58e2 to
9154951
Compare
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
9154951 to
318a10e
Compare
PerfectSlayer
approved these changes
Aug 12, 2025
Comment on lines
+65
to
+78
| private static final DDCache< | ||
| String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>> | ||
| PEER_TAGS_CACHE = | ||
| DDCaches.newFixedSizeCache( | ||
| 64); // it can be unbounded since those values are returned by the agent and should be | ||
| // under control. 64 entries is enough in this case to contain all the peer tags. | ||
| private static final Function< | ||
| String, Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>> | ||
| PEER_TAGS_CACHE_ADDER = | ||
| key -> | ||
| Pair.of( | ||
| DDCaches.newFixedSizeCache(512), | ||
| value -> UTF8BytesString.create(key + ":" + value)); |
There was a problem hiding this comment.
🎯 suggestion: Can't we make a dedicated type for the cache and creator rather than using a Pair<DDCache<String, UTF8BytesString>, Function<String, UTF8BytesString>>? It feels hard to read.
Because it always end up calling cache.computeIfAbsent(key, creator), so the whole Pair<...> thing can be simplified as a functional interface like: Function<String, UTF8BytesString>.
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
318a10e to
329a2e5
Compare
Also delays the creation of the Utf8ByteString at serialization time. Note that `writer.writeUTF8` emits a header corresponding to the length of the value being written, calling this method again will count as an entry in the array. A possible idea was to use a cache to store the computation of the peerTags, however `Utf8ByteString` is not concatenable / appendable, which is necessary to have the proper encoding. Creating a new "temporary" `Utf8ByteString`, was replaced by a direct call to `String: :getBytes`.
* Add jmh for metrics aggregation * Cache peer tags to avoid too many strings/utf8 conversions * Use span kind cache * Fix tests
amarziali
force-pushed
the
bdu/add-missing-attributes-in-metrics-key
branch
from
812a250 to
34da211
Compare
bric3
commented
Aug 27, 2025
| @@ -311,7 +311,7 @@ private boolean publish(CoreSpan<?> span, boolean isTopLevel) { | |||
| span.getType(), | |||
| span.getHttpStatusCode(), | |||
| isSynthetic(span), | |||
| span.isTopLevel(), | |||
| span.getParentId() == 0, | |||
There was a problem hiding this comment.
suggestion: Shouldn't we use DDSpanId.ZERO ?
| @@ -118,7 +118,7 @@ public void add(MetricKey key, AggregateMetric aggregate) { | |||
| writer.writeBoolean(key.isSynthetics()); | |||
|
|
|||
| writer.writeUTF8(IS_TRACE_ROOT); | |||
| writer.writeBoolean(key.isTraceRoot()); | |||
| writer.writeInt(key.isTraceRoot() ? 1 : 2); // tristate (0 unknown, 1 true, 2 false) | |||
There was a problem hiding this comment.
suggestion: Might be useful to represent the tri state by constants
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
This fills the gap regarding peer tags, by declaring the in the
MetricKey.Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]